fix: Added date time parsing for conditional formatting #1120
Conversation
| // Proper date validation will be addressed by Issue #1108 | ||
| return value != null && value !== ''; | ||
| default: { | ||
| const [dt, tz] = (value ?? '').split(' '); |
There was a problem hiding this comment.
One suggestion - use camel case and full words for variable names. We try to follow the Airbnb style guide – https://github.com/airbnb/javascript#naming-conventions
with a few exceptions. e as the catch block argument is fine, single-character names in loops and maps are also fine, as long as the loop body is fairly short.
In this case, these are local variables and the scope is short so it's probably fine, but I thought I'd mentioned this anyway.
Codecov Report
@@ Coverage Diff @@
## main #1120 +/- ##
==========================================
+ Coverage 43.37% 43.39% +0.01%
==========================================
Files 434 434
Lines 32617 32626 +9
Branches 8218 8218
==========================================
+ Hits 14148 14157 +9
Misses 18420 18420
Partials 49 49
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| // Proper date validation will be addressed by Issue #1108 | ||
| return value != null && value !== ''; | ||
| default: { | ||
| const [dateTimeString, tzCode] = (value ?? '').split(' '); |
There was a problem hiding this comment.
Would 2023-02-23T11:46:31.000000000 NY FOO pass validation but still fail server size?
There was a problem hiding this comment.
Good catch.
Also, a dateTime string can have a space in place of the t-separator: 2023-02-23 11:46:31.000000000 NY.
Regex for the separator in DateUtils/parseDateTimeString looks like this: [tT\s].
There was a problem hiding this comment.
Ignore my comment, looks like the engine expects a T in format conditions.
There was a problem hiding this comment.
It would be nice if we could accepted strings anywhere in the UI that takes a datetime with or without the T separator, then normalize it before sending.
There was a problem hiding this comment.
Agree, also accept datetime with or without the time zone. Should default to the time zone in the user settings. That would be another ticket though.
bmingles
force-pushed
the
1108-conditional-date-formatting
branch
from
a332ccf
to
cc2c1c2
Compare
|
@vbabich It was suggested by @niloc132 that instead of using I'm hesitant to do that for this issue given the current solution is already implemented, but I can create a new issue if we think this is a better solution. |
|
@bmingles Agreed, open another issue for this. |
Added datetime string parsing / validation.
fixes #1108